This repository has been archived by the owner on May 10, 2024. It is now read-only.
Karkakol/add active simulcast encodings #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # MembraneRTC/src/main/java/org/membraneframework/rtc/InternalMembraneRTC.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/RTCEngineCommunication.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/RTCEngineListener.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/SimulcastConfig.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/models/Endpoint.kt
# Conflicts: # MembraneRTC/src/main/java/org/membraneframework/rtc/InternalMembraneRTC.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/RTCEngineListener.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/events/Event.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/models/Endpoint.kt # MembraneRTC/src/main/java/org/membraneframework/rtc/models/TrackContext.kt
karkakol
requested review from
incubo4u and
graszka22
and removed request for
incubo4u
graszka22
reviewed
Jan 16, 2024
.idea/gradle.xml
Outdated
| <option name="externalProjectPath" value="$PROJECT_DIR$" /> | ||
| <option name="gradleJvm" value="Embedded JDK" /> | ||
| <option name="gradleJvm" value="#GRADLE_LOCAL_JAVA_HOME" /> |
There was a problem hiding this comment.
that's weird, is it needed?
| @@ -7,13 +7,15 @@ | |||
| </inspection_tool> | |||
| <inspection_tool class="PreviewApiLevelMustBeValid" enabled="true" level="ERROR" enabled_by_default="true"> | |||
| <option name="composableFile" value="true" /> | |||
| <option name="previewFile" value="true" /> | |||
There was a problem hiding this comment.
that's also weird, also is it needed?
There was a problem hiding this comment.
you are right, .idea shoud be git ignored
| import org.membraneframework.rtc.utils.Metadata | ||
|
|
||
| data class Endpoint( | ||
| val id: String, | ||
| val type: String, | ||
| val metadata: Metadata? = mapOf(), | ||
| val trackIdToMetadata: Map<String, Metadata?> = mapOf() | ||
| val trackIdToMetadata: Map<String, Metadata?> = mapOf(), | ||
| val tracks: Map<String, TracksAdded.Data.TrackData> |
There was a problem hiding this comment.
here too we have trackIdToMetadata and tracks, if it's user facing class it would be confusing
There was a problem hiding this comment.
Also an endpoint might be a local endpoint, do we update it in that case?
There was a problem hiding this comment.
if it's user facing it doesn't look nice to me (the TracksAdded.Data.TrackData type is too long and TracksAdded event is not user facing. Consider creating a new type maybe?
There was a problem hiding this comment.
unnecessary data is removed
graszka22
reviewed
Jan 16, 2024
MembraneRTC/src/main/java/org/membraneframework/rtc/models/TrackContext.kt
Show resolved
Hide resolved
graszka22
reviewed
Jan 18, 2024
| import org.membraneframework.rtc.SimulcastConfig | ||
| import org.membraneframework.rtc.utils.Metadata | ||
|
|
||
| data class TrackData(val metadata: Metadata?, val simulcastConfig: SimulcastConfig?) |
There was a problem hiding this comment.
You may document this too if it's user-facing
graszka22
approved these changes
Jan 25, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.